Add image type for picture-elements card.#1428
Conversation
| import '../../../components/buttons/ha-call-service-button.js'; | ||
| import '../../../components/entity/ha-state-label-badge.js'; | ||
| import '../../../components/entity/state-badge.js'; | ||
| import '../components/hui-image.js'; |
There was a problem hiding this comment.
Please order imports by depth. This is a local Lovelace import, but it at the bottom.
| return html` | ||
| <style> | ||
| :host { | ||
| overflow-y: hidden; |
There was a problem hiding this comment.
If this is because of it being placed in entity-picture, the CSS should be added to entity-picture. We should never add CSS to web components because it helps styling it in a parent. The parent should contain the styling instead.
There was a problem hiding this comment.
This became apparent because of picture-elements, but I think it is the correct behavior in general. If the parent has a static height I feel like the expectation is that the image won't overflow. But the only place this is a factor right now is picture-elements, so I'll move this css there and reassess if it comes up other places too
|
The name should be state-image for consistency. MAYBE we can also use one click handler for all elements |
|
One click handler per element is a lot easier to reason about. |
|
I added a separate one for images because images allow service calls where the others don't. We could allow service calls for the other element types too, or check the type in the handler if it's trying to call a service. I'll go with whatever solution you guys decide on though |
|
Also, @c727 I didn't use state-image because I was concerned people would get confused if they weren't trying to setup state_images. I will change it if you'd rather keep the naming consistent for all elements though |
|
yep, I thought someone might also wants to use servicecalls on the other state-elements instead of toggle/dialog, but we can also change this later. If I suggest changes I'm always open for discussion, not every suggestion must be good :P |
|
You have done a lot more work on lovelace than me so I like to defer to your judgement, but you have to fight it out with balloob 😄 |
|
We can always add a helper method, |
|
@balloob are you talking about a helper to add a separate click handler? Something like https://hastebin.com/oyuyeyifox.javascript |
|
Yep. That way we don't need to implement that method for every element and enforce the same config |
|
I originally went with the helper, but it seemed really redundant. We need click handlers on 3 of the element types which are the same 3 that the helper would have been wrapping since the other 3 types handle clicks differently, so I just put the service call logic in the click handler. I will change it if you still think it would be better |
|
Looks good. |
Depends on #1423
Allows adding images to a picture-elements card
Closes: home-assistant/ui-schema#75
Example using
state_imageandstate_filter:Example using camera and service call: